- 
                Notifications
    
You must be signed in to change notification settings  - Fork 6
 
Change ctl check to not use InfrahubCheck.init() #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
          Codecov ReportAttention: Patch coverage is  
 
 @@             Coverage Diff              @@
##           develop     #126       +/-   ##
============================================
- Coverage    65.38%   44.43%   -20.96%     
============================================
  Files           76       76               
  Lines         6917     6922        +5     
  Branches      1367     1296       -71     
============================================
- Hits          4523     3076     -1447     
- Misses        2026     3566     +1540     
+ Partials       368      280       -88     
 Flags with carried forward coverage won't be shown. Click here to find out more. 
  | 
    
        
          
                infrahub_sdk/checks.py
              
                Outdated
          
        
      | async def init(cls, client: Optional[InfrahubClient] = None, *args: Any, **kwargs: Any) -> InfrahubCheck: | ||
| """Async init method, If an existing InfrahubClient client hasn't been provided, one will be created automatically.""" | ||
| warnings.warn( | ||
| "InfrahubCheck.init has been deprecated and will be removed in the version in Infrahub SDK 1.1.0", | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code will ship for the first time with 1.1 so this can't be accurate
To be clean let's say 2.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to get rid of the InfrahubClient() call below as it requires us to specifically import InfrahubClient. Are you fine with saying "after 1.1.0" instead? Alternatively we can change it to 2.0 and use importlib or similar here until that point. I don't think anyone outside of infrahubctl or the git-agent ever used this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed an update to schedule this method to be removed in SDK 2.0 instead and at the same type we change how InfrahubClient is used within that module in order to change the InfrahubCheck module to follow the same logic as used for Transforms and generators in #128.
674ad9c    to
    7031c18      
    Compare
  
    7031c18    to
    e231c93      
    Compare
  
    
This work relates to opsmill/infrahub#3932 as a goal to have a consistent way of loading modules that behaves the same way. An issue that might come up is that of circular imports as the check.py is importing InfrahubClient.
We're removing the use of InfrahubCheck.init() from the CTL as it doesn't provide much value and also marking that method as deprecated.